-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement a query manager for running queries #5950
Conversation
Yes, although it doesn't have anything related to multi-host support at the moment. If you want to kill a query, you have to run it against that specific node. |
a662a19
to
0f35d45
Compare
9149de4
to
6dd78e4
Compare
s += fmt.Sprintf("%du", d) | ||
} | ||
return s | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't time.Duration.String()
do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't output microseconds correctly, but I can make the function a lot more simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@jsternberg Why is there an |
I'll try it out. I can't remember if I tried that or not. |
The reason it was done this way was so that a channel could be closed from one of a few different places and it would automatically close anybody who was subscribed to that channel. It means I could just pass the channel into each iterator. The interrupt has to be done at multiple points in the engine so there need to be several places where the interrupt is checked. I couldn't really find a way to interrupt in the middle of a call to
Will only return a single point from a call to That's the rationale for the interrupt iterator. |
6dd78e4
to
f1ca4f0
Compare
@jsternberg I can understand needing to use a lock or a channel at some point lower down in the iterator chain to stop the lower |
ea59346
to
d541d4d
Compare
@benbjohnson I don't think that interpretation is correct. The channel is used as a side-channel to interrupt the iterator. The I think of it as more similar to how Unix signals are implemented and the interrupt iterator is the part that handles the signal interrupt. I've been trying to do it with close and it involves spawning a new goroutine at the top level and ensuring that all |
The You can still implement something like the interrupt iterator that handles the |
I don't think it's wise to conflate the interrupt operation with a close operation. A close operation is designed to clean up resources while an interrupt is usually an asynchronous interruption that is easily facilitated through channels. I used the method for how interrupts are implemented with Java threads here instead of trying to make interrupts use the close method: https://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html I also think it follows with the same idea of how Unix signals and process management are done on Linux. An interrupt is a request that the iterator (or process in Linux) be closed, not a demand. I also think there's another problem with using
The problem is that the two closes don't know which is which. They're different operations. I also think it works better with Go idioms. Channels are meant for different goroutines to communicate with each other. We would be sharing memory between multiple goroutines by having |
I think you're overcomplicating the The Adding an interrupt channel means that we have to pass it down in the |
The currently running queries can be listed with the command `SHOW QUERIES` and it will display the current commands that have been run, the database they were run against, and how long they have been running.
While this allows a query to be killed, it doesn't really do anything yet since the interrupt happens only after the first row gets emitted (the entire first series). This section of code will likely have to be refactored to make this work since we need a way to interrupt a currently running iterator.
…an interrupt Use of the iterator is spread out into both `IteratorCreators` and inside of the iterators themselves. Part of the interrupt must be handled inside of the engine so it stops trying to emit points when an interrupt is found and another part of the interrupt has to happen when combining the iterators so it doesn't just start reading the next shard.
This seems to have been an oversight since all of the response writers are supposed to implement this interface, but the gzipResponseWriter didn't implement this interface for some reason.
8cc0139
to
e5cebd2
Compare
I've renamed the |
I know MySQL has |
|
e5cebd2
to
17c9fa5
Compare
Fixes #675 |
17c9fa5
to
504bac5
Compare
I'm removing the timeout related stuff from this PR so it can be part of a separate PR. |
Implement a query manager for running queries
Fixes #5939.